Review: perf: shrink generated TestEntry builder IL via shared TUnit.Core factory helpers

Overall: This is a well-designed, well-executed performance optimization. Approved with minor notes.

The design is sound — two complementary changes that reduce generated IL substantially (-11k/+7k lines) by:

1. Moving per-call-site async iterator state machines into shared DataSourceFactories helpers
2. Replacing 17-property object initializers with a single TestEntryFactory.Create<T> factory call using named arguments

Both strategies are coherent and the AOT/trim constraints are respected throughout.

Issue 1: #pragma warning disable CS1998 at file scope (minor)

In DataSourceFactories.cs, the CS1998 suppression covers the entire file:

// top of file
#pragma warning disable CS1998

Only the four non-awaiting methods (FromValue, FromEnumerable, FromInstanceValue, FromInstanceEnumerable) need this. If a future method is added to this file with a genuine missing await, the warning would be silently suppressed. Prefer per-method suppressions:

#pragma warning disable CS1998 // intentionally sync-only async iterator
public static async IAsyncEnumerable<...> FromValue(...)
#pragma warning restore CS1998

Why it matters: This file is the canonical home for data source iteration logic. Suppressing the whole file risks hiding real async mistakes as the class grows.

Issue 2: ValueTask detection via string prefix inside IsTask branch (minor design smell)

IsTask() already returns true for both Task<T> and ValueTask<T> (by checking string prefixes). Inside its branch, a second StartsWith check distinguishes them:

else if (IsTask(returnType))
{
    kind = returnType.ToDisplayString().StartsWith("System.Threading.Tasks.ValueTask")
        ? "ValueTask"
        : "Task";
}

This is consistent with the existing IsTask implementation, but the pattern exposes that IsTask is doing double duty. A cleaner approach:

// Split the check
else if (IsValueTask(returnType))
{
    kind = "ValueTask";
}
else if (IsTask(returnType))
{
    kind = "Task";
}

where IsValueTask does the StartsWith("System.Threading.Tasks.ValueTask") check and IsTask is updated to only match Task<T>. This eliminates the double-check without changing behavior, and makes the dispatch table read like a flat decision tree rather than a nested override.

Not a blocker — the current code is correct — but worth a note since EmitDataSourceFactoryInvocation is now the only caller that needs to distinguish them, and refactoring IsTask fixes it permanently.

Things done well

• Factory argument order in TestEntryFactory.Create is excellent: required high-frequency args first, optional defaults last. The default array parameters using null (instead of Array.Empty<T>()) is the right choice — it avoids allocations at the generated call sites while the factory consolidates the null-coalescing in one place.
• #if !DEBUG [EditorBrowsable(EditorBrowsableState.Never)] #endif pattern on both new public types keeps IntelliSense clean without hiding helpers during debugging.
• GetRequiredInstance extraction in DataSourceFactories avoids repeating the null-check + error message across all instance variants — clean.
• Test coverage is thorough: all 4 TFMs re-verified, snapshot scrub lists extended for the new named-argument forms, and BasicTests column assertions correctly made conditional since column spans are environment-sensitive.
• EmitDataSourceFactoryInvocation correctly handles the block-body form for argument-bearing data sources (where arguments must be bound before the invocation), and the single-expression form for the common no-argument case — the generated code is readable in both paths.

The two issues above are improvements worth making but neither is a blocker. The performance motivation is solid and the implementation is correct across all documented test paths.

Follow-up review: issues from prior review already addressed

Looking at the actual patch in this PR, both concerns raised in the previous review comment are already handled in the submitted code:

Issue 1 (CS1998 suppression scope): The patch uses targeted per-block suppressions rather than file-scope. The #pragma warning disable CS1998 / #pragma warning restore CS1998 pairs wrap only the four sync-only async iterator methods (FromValue, FromEnumerable, FromInstanceValue, FromInstanceEnumerable). The concern about silently masking future async mistakes does not apply here.

Issue 2 (IsTask/IsValueTask dispatch): The patch already introduces a dedicated IsValueTask method and restricts IsTask to only System.Threading.Tasks.Task prefix. The generator's type dispatch is now a clean flat decision tree: IsAsyncEnumerable → IsValueTask → IsTask → IsEnumerable → single value. The double-check concern is fully resolved.

No blocking issues. The two new public types (DataSourceFactories, TestEntryFactory) are well-designed: they are static, AOT-safe, correctly annotated with [DynamicallyAccessedMembers], and hidden from IntelliSense in non-debug builds. The IL reduction (~-11k / +7k generated lines) is a genuine win. Approving.

The implementation is correct and the two concerns from the previous review comment are already addressed in this PR. Approving.